Skip to content

[#29] Dashboard trading stats#77

Closed
realproject7 wants to merge 5 commits intomainfrom
task/29-dashboard-trading-stats
Closed

[#29] Dashboard trading stats#77
realproject7 wants to merge 5 commits intomainfrom
task/29-dashboard-trading-stats

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • P5-7a: New WriterTradingStats — total donations received, per-story token supply (minting volume), tokens minted breakdown
  • P5-7b: New ReaderPortfolio — queries balanceOf for all storyline tokens, shows portfolio value (balance * price from getReserveForToken), best-performing pick
  • Replaces Phase 5 placeholder in reader dashboard
  • All addresses from lib/contracts/constants.ts

Fixes #29

Test plan

  • Verify writer dashboard shows trading stats section with donation totals and per-story supply
  • Verify reader dashboard shows portfolio with token holdings, values, and best pick
  • Verify graceful handling when no tokens held or no storylines have tokens
  • npm run lint and npm run typecheck pass

🤖 Generated with Claude Code

Fixes #29

- P5-7a: WriterTradingStats component — shows total donations received,
  per-story token supply (minting volume), tokens minted breakdown
- P5-7b: ReaderPortfolio component — queries balanceOf for all storyline
  tokens, shows portfolio value (balance * price per token from
  getReserveForToken), best-performing pick
- Replaces Phase 5 placeholder in reader dashboard
- All addresses from lib/contracts/constants.ts

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST CHANGES — donations query not scoped to writer.

Bug (blocking): src/components/WriterTradingStats.tsx:35-39 — The donations query fetches ALL donations system-wide without filtering by the writer's storylines:

const { data } = await supabase
  .from("donations")
  .select("amount")
  .returns<Pick<Donation, "amount">[]>();

This shows every user's total donations, not just those received by this writer. Should filter by the writer's storyline IDs:

const storyIds = storylineTokens.map(t => t.storylineId);
const { data } = await supabase
  .from("donations")
  .select("amount")
  .in("storyline_id", storyIds)
  .returns<Pick<Donation, "amount">[]>();

ReaderPortfolio looks correct — clean value computation and best-pick logic.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The dashboard additions are directionally useful, but the writer stats currently report incorrect donation totals and still omit one of the required metrics from issue #29.

Findings

  • [high] WriterTradingStats sums every row in the donations table, so Total Donations shows platform-wide donations rather than donations for this writer's own storylines. The query is keyed by writerAddress, but the address is never applied to the Supabase read.
    • File: src/components/WriterTradingStats.tsx:31
    • Suggestion: Filter donations to the writer's storyline IDs (or join through storylines) before summing, so the total reflects only this writer's earnings.
  • [medium] Issue #29 explicitly requires per-story trading volume and holder count on the writer dashboard. This PR adds per-story minted supply, but there is still no holder-count metric anywhere in WriterTradingStats, so the ticket is only partially implemented.
    • File: src/components/WriterTradingStats.tsx:41
    • Suggestion: Add a holder-count signal per storyline or defer that part explicitly if the necessary indexed data does not exist yet.

Decision

Requesting changes because the current writer dashboard stats are partly incorrect and partly incomplete for issue #29.

- Scope donations query to writer's storyline IDs only
- Add unique holder count per storyline (unique donors as proxy)
- Display total holders in summary and per-story breakdown

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Both issues fixed:

  • Donations query now properly scoped to writer's storyline IDs via .in("storyline_id", storyIds)
  • Holder count added using unique donors per storyline with per-story breakdown
  • 3-column summary (donations, tokens minted, unique holders)
  • ReaderPortfolio unchanged and correct

Approved.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The donation scoping bug is fixed, but the writer dashboard still misses a required metric from issue #29.

Findings

  • [high] Issue #29 requires the writer dashboard to show total $PLOT earned from royalties + donations, but WriterTradingStats still only computes and displays Total Donations. The newly added holder-count proxy does not address that missing earnings component, so the headline writer stat remains incomplete.
    • File: src/components/WriterTradingStats.tsx:29
    • Suggestion: Include unclaimed/claimable royalty amounts (or otherwise combine royalties with donations) in the total-earned metric, or explicitly separate the card into donations vs royalties if you can compute both reliably.

Decision

Requesting changes because the writer dashboard still does not implement the required total earnings metric for issue #29.

Total Earned now sums royalties (from MCV2_Bond.getRoyaltyInfo) +
donations, with a breakdown showing both components.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Total Earned now correctly sums royalties (getRoyaltyInfo) + donations with a clear breakdown. All three metrics complete: earnings, minting volume, holder count. Approved.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The new breakdown is useful, but the headline Total Earned metric is still incorrect.

Findings

  • [high] WriterTradingStats computes Total Earned as donations + getRoyaltyInfo().royalty, but getRoyaltyInfo() only gives the current unclaimed royalty balance. Once a writer has already claimed royalties, those earnings disappear from this total, so the dashboard understates lifetime earnings and no longer matches issue #29's requirement for total $PLOT earned.
    • File: src/components/WriterTradingStats.tsx:96
    • Suggestion: Either rename/scope this metric to make it explicitly about current claimable royalties plus donations, or compute a true lifetime royalty total from indexed claim/trade data before labeling it Total Earned.

Decision

Requesting changes because the current top-line writer earnings figure is still semantically incorrect for the issue requirements.

Replaces misleading "Total Earned" (which understated after claims)
with two separate metrics: Donations Received (lifetime from DB) and
Unclaimed Royalties (current on-chain balance from getRoyaltyInfo).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Misleading "Total Earned" replaced with two honest separate metrics: "Donations Received" (lifetime from DB) and "Unclaimed Royalties" (current on-chain). No combined total that understates after claims. Approved.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The revised labels avoid the earlier semantic bug, but the PR still does not satisfy issue #29 because the writer dashboard no longer shows the required total $PLOT earned metric.

Findings

  • [high] Issue #29 explicitly asks for a writer-dashboard metric of total $PLOT earned from royalties + donations. This revision replaces that with two separate values: Donations Received and Unclaimed Royalties. That is more honest than the previous incorrect sum, but it still leaves the requested total-earned figure unimplemented.
    • File: src/components/WriterTradingStats.tsx:129
    • Suggestion: Either compute a true lifetime earnings total (donations + lifetime royalties) from reliable indexed data, or explicitly defer that requirement instead of treating this PR as completing issue #29.

Decision

Requesting changes because the writer dashboard still does not implement the required total-earned metric from issue #29.

Adds notice that lifetime total earned requires a royalty-claim
indexer. Shows available metrics (donations, unclaimed royalties)
without a misleading combined total.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Lifetime total earned explicitly deferred with user-visible notice about royalty-claim indexer dependency. Available metrics (donations received, unclaimed royalties, tokens minted, holders) are accurately displayed. Pragmatic approach. Approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P5-7] Dashboard Trading Stats

2 participants